Is an XL Dialog needed here? It seems like the a smaller dialog may be more appropriate.

Changed this to a LARGE_DIALOG.

The dialog size looks great. Can we update it so the content shares the available space like other dialogs (for instance Edit Controller Service)?

I fixed this. It now extends all the way.

This table is used for both services in the Flow as well as services in the Controller Settings. This is why the ControllerServiceTable is in the common components. Since Services in the Controller Settings do not exist within a Process Group, this action is not be available.

Made the move option not show up in management controller services.

Thanks for the update here. The new Input() supports function that returns whether the service can be moved. However, the implements are not service specific and always just return true or false. Can this Input() be simplified into a boolean?

@Input() canMove: boolean = false;

Changed this to just be a boolean.

The state for the flow-designer page is already loaded with the current process group. We should be able to simply use that instead of making another request here.

The current state did not store the entire flow. I did change this and I was able to use it here without an api call. However, with the implementation of disabled drop down menu process groups, I did need to 2 other api calls here.

Ah sorry. Yes you are correct. I thought we had stashed the entire processGroupFlow in the service state as well but I see that only saved the breadcrumbs and parameter context details. Now we're saving the entire processGroupFlow next to the breadcrumbs and parameter context details.

Do we only need the processGroupFlow in order to get the child Process Groups for the Move dialog? If so, what are your thoughts only saving them in the store. Often times the flow can be very large and in this page we won't need most of those details.

Let me know if I missed something and we do need additional details.

Thank you for the idea, I hadn't thought of that. I removed the storing of the whole process group flow, and just saved an array with names and IDs.

getProcessGroupById will return null for a couple different scenarios. When this happens calling processGroupContainsComponent will error with the following. The unhandled error leaves the UI in a weird state because the overlay is active but the dialog isn't shown.

TypeError: Cannot read properties of null (reading 'contents')
    at _MoveControllerService.processGroupContainsComponent (move-controller-service.component.ts:184:26)
    at move-controller-service.component.ts:166:27
    at Array.forEach (<anonymous>)
    at _MoveControllerService.loadChildOptions (move-controller-service.component.ts:157:34)
    at new _MoveControllerService (move-controller-service.component.ts:99:14)
    at NodeInjectorFactory.MoveControllerService_Factory [as factory] (move-controller-service.component.ts:231:5)
    at getNodeInjectable (core.mjs:6060:44)
    at createRootComponent (core.mjs:17231:35)
    at ComponentFactory.create (core.mjs:17091:29)
    at ViewContainerRef2.createComponent (core.mjs:17498:47) 

It appears this was happening when childProcessGroupOptions contained more than one option. The recursive logic in getProcessGroupById (and other methods with similar structure) isn't correct. When there are multiple children the following code

                    for (const pg of root.contents.processGroups) {
                        return this.getProcessGroupById(pg, processGroupId);
                    }

will return the result from the first child and not attempt to process any subsequent children.

I've fixed the recursion logic.

Thanks for fixing the recursion issue. However getProcessGroupById returns null in a couple scenarios. That response value is used as though it's guaranteed non-null. We should protect against this.

Done.

Done.

Under what conditions is request.breadcrumb undefined? I think the store has an initial value but even if that isn't the case we could update the concatLatestFrom with the following:

this.store.select(selectBreadcrumb).pipe(isDefinedAndNotNull())

I was trying to only use a single request model, but I've created 2 and removed the need to declare it as breadcrumb?

Why is a clone being created here?

Same as above. Removed need to clone.

I think this was left in inadvertently.

It was. I've removed it.

Thanks for fixing the recursion issue. However getProcessGroupById returns null in a couple scenarios. That response value is used as though it's guaranteed non-null. We should protect against this.

This button should be disabled when the form is not valid. For instance, when only disabled options are present and the user cannot make a selection.

The button is now disabled when there no valid options.

Once the Submit button is disabled, I think we should be able to remove this check here as the form would never be submitted.

Removed.

Thoughts on splitting the dialog space, half for the Group selection and half for the References?

I agree it looks better. Done.

Please move the move-controller-service folder under src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service.

I've moved this to ui/controller-service

Can you use the state of the form instead of introducing a new field? I didn't try this out but maybe something like the following:

I've changed this to use the state of the form.

Process Group is not a valid value for this field. We could initialize the value to null but we're setting the value below. We could instead determine the value before creating the form and then just use the correct value when creating the FormControl.

Not sure what you mean by this. "Process Group" is what is shown whenever there is no process group selected. The only time this happens is if there are no valid process groups available.

The first argument to the FormControl is the value. The string Process Group is not a valid value. What you see in the control when no value is select comes from the mat-label in the mat-form-field. If you want something else than the mat-label value you can specify a placeholder on the mat-select.

Please convert to a template string.

No longer used.

Please convert to a template string if possible.

No longer used.

No longer used.

It may make this more readable if we swap the if/else blocks and update the conditional to:

No longer used.

No longer used.

No longer used.

This change is necessary for this feature to ensure that when moving a Controller Service it remains in scope for all components currently referencing it. However, I have some concerns about fetching the entire flow from the current Process Group when the Move dialog is opened. I wonder if we should consider a purpose built endpoint for does exactly what we need instead of returning everything. We've certainly seen some large flows over the years where I worry this could be problematic.

CC @exceptionfactory

I've created a new API endpoint, move-options, to get the information needed for the process groups. This shifts all the logic to the API, and greatly simplifies the UI form.